chore: 📝 Links in deflist in Rocqnavi comments - #2103
yoshihiro503 wants to merge 1 commit into
Conversation
|
@hoheinzollern @proux01 RFC: What do you think about this "feature"? This is low-tech but arguably more stable than adding custom parsing, not very user-friendly but anyway we do not expect use to write tons of documentation. |
|
sounds good |
|
looks good, I'm a bit worried though about stuff like |
|
it might also not be stable across edits (have you checked?) |
The hash is created by a standard OCaml function so that should be fairly stable.
We need to think about that indeed @yoshihiro503 |
|
you can consider also a flag that fails to build the documentation if the link does not match a symbol in the file, this way you can catch any mismatch, especially for the hash-based ones. |
|
Unfortunately, there is no "building" the documentation, it's just plain markdown interpreted by the browser when you view it. Maybe there exist tools to check links in there though? |
|
@proux01, I mean rocqnavi builds the html page and generates the links, it can check that the anchors in the markdown sections are generated by the html rendering of the code, with a warning by default and an optional flag that fails the build if the check fails. |
|
Sure, but that's not technically trivial since currently the markdown comment is not really parsed by rocqnavi, only copied to the html page (the markdown interpretation being done in the browser by some javascript code). |
|
Agreed, though I think it's just a regular expressions to identify internal anchors of the form Alternative idea: if our flavor of markdown supports language-specific highlighting (e.g., like in pandoc: |
|
The anchor check could also be done with an HTML/Markdown validator at the end of the build. |
|
Also, I checked out the sample artifact, and noticed that the link to the notation for the variance points to the wrong anchor (the local notation inside the section building it, not the global instance). This is not a great process and requires a lot of manual review. Digging a bit, I can see that the hashes are built from the .glob files, e.g., |
|
From a quick experiment across the entire MC/MCA repos, we can entirely bypass the hashing and just use the output identifiers from the .glob files, e.g.: with a few caveats that I'm aware of:
Advantages:
Disadvantages:
Yet another approach is to derive an simpler identifier, e.g.: I'm leaning against this options as it would further complicate the parser, we would need to be absolutely sure that the encoding is injective, and I'd rather push a change like this upstream to the .glob file generation if we think it's worthwhile. (I'm worried though than an encoding like this is not injective) |
|
I don't know, I feel the hashes are nearly less disturbing when reading the source. I fear your proposition may make the links harder to copy/quote... Anyway, that's not a strong opinion, feel free to ignore it, but I feel the current PR offer is fine for now. Checking links would be nice but can be done separately. |
Motivation for this change
probability_theory/random_variable.v
Checklist
CHANGELOG_UNRELEASED.mdReference: How to document
Merge policy
As a rule of thumb:
all compile are preferentially merged into master.
Reminder to reviewers